Skip to content

Conversation

LoganBarnett
Copy link

The project I work on has a requirement for 100% type coverage. When making my own custom validators, I noticed properties hanging off context weren't something Flow could infer (they become any). Adding this return type is enough for Flow to achieve type coverage with usage of context.

@andreypopp
Copy link
Owner

Is it still relevant? (I know flow also fixed some bugs with coverage reporting).

@LoganBarnett LoganBarnett force-pushed the add-flow-coverage-pr branch from afb582e to 6507227 Compare April 17, 2018 22:47
@LoganBarnett
Copy link
Author

@andreypopp sorry for disappearing! It appears to still be relevant. I'm trying to put together a simple reproduction here. I'm running into a problem though with $NonMaybeType:


✎ yarn flow
yarn run v1.3.2
warning package.json: No license field
$ /Users/logan/dev/validated-context-coverage/node_modules/.bin/flow
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/validated/lib/schema.js.flow:158:45

Missing type annotation for $NonMaybeType.

     155│   }
     156│ }
     157│
     158│ export class AnyNode<V: mixed> extends Node<$NonMaybeType<V>> {
     159│   validate(context: Context): ValidateResult<$NonMaybeType<V>> {
     160│     return context.unwrap(value => {
     161│       if (value == null) {

I'm a little puzzled at the moment and haven't been able to put a lot of cycles towards addressing it yet. I suspect it has to do with a recent version of Flow (currently 0.70.0). I will try downgrading and seeing if I can move forward from there.

@LoganBarnett
Copy link
Author

Downgrading to flow-bin 0.69.0 got me moving. My fix doesn't seem to work given my reproduction, so I'll need to fix that. You can see the problem on master validated-context-coverage. If you check out coverage-fix and run yarn or npm install you can see the fix not working. I'll be working on that part ;)

@LoganBarnett
Copy link
Author

Okay I've updated the PR that adds some type coverage that ultimately provides the needed coverage for the test repo to work at full coverage as well. This is ready again for review.

@@ -28,7 +28,8 @@ export type ValidateResult<V> = {
+value: V,
};

type Refine<A, B> = (value: A, error: (message: GenericMessage) => void) => B;
// Maybe this should be Error instead of mixed?
type Refine<A, B> = (value: A, error: (message: GenericMessage) => mixed) => B;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what you think here. Error maybe?

@LoganBarnett LoganBarnett force-pushed the add-flow-coverage-pr branch from e96d169 to 4a9bd9a Compare April 18, 2018 22:51
@LoganBarnett LoganBarnett force-pushed the add-flow-coverage-pr branch from 3cd2942 to f338de3 Compare March 6, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants